Skip to content

Conversation

@gribnoysup
Copy link
Collaborator

@gribnoysup gribnoysup commented Aug 7, 2025

This patch changes the drawer title behavior in data-modeling to show a more specific label and also moves some drawer item related controls to the heading section. For now it's just one item that we show for relationships, but this will be expanded in the future, so I already tried to account for that

image image

…awer header; show item associated actions in header
@gribnoysup gribnoysup changed the title chore(data-modeling): show collection name / relationship label in drawer header; show item associated actions in header chore(data-modeling): show collection name / relationship label in drawer header; show item associated actions in header COMPASS-9653 Aug 7, 2025
@gribnoysup gribnoysup changed the title chore(data-modeling): show collection name / relationship label in drawer header; show item associated actions in header COMPASS-9653 feat(data-modeling): show collection name / relationship label in drawer header; show item associated actions in header COMPASS-9653 Aug 7, 2025
@gribnoysup gribnoysup added the feature flagged PRs labeled with this label will not be included in the release notes of the next release label Aug 7, 2025
@github-actions github-actions bot added the feat label Aug 7, 2025
@GaurabAryal GaurabAryal requested a review from Copilot August 7, 2025 17:07

This comment was marked as resolved.

@gribnoysup gribnoysup marked this pull request as ready for review August 8, 2025 10:12
@gribnoysup gribnoysup requested a review from a team as a code owner August 8, 2025 10:12
// TODO(ticket): This is obviously a horrible selector and we should make sure
// that LG team provides a better one for us to achieve this behavior when
// we're removing the vendored version of the drawer
'& > div:nth-child(2) > div:nth-child(2) > div:first-child > div:first-child > div:first-child > div:first-child':
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So a slightly less hacky version of this that I had initially was div:has(>strong), but our version of jsdom can't handle it and I didn't want to include version bump in this PR, but I'll revisit this after

// layout the controls there better. Doing our best to target the section
// title here, leafygreen really doesn't give us anything else to try.
//
// TODO(ticket): This is obviously a horrible selector and we should make sure
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hoping to get a ticket number from this Slack thread

Comment on lines +42 to +48
const { id } = getActiveDrawerContent() || {};
const lgIds = getLgIds(dataLgId);
const hasData = toolbarData && toolbarData.length > 0;
const { title, content } =
toolbarData.find((data) => {
return data.id === id;
}) ?? {};
Copy link
Collaborator Author

@gribnoysup gribnoysup Aug 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is like a super minimal version of the fix that LG team did in this patch, mapping their changes on top of our vendored package was tricky due to naming (and seems like code was generally shifted around more there), but we can be sure that when this is not vendored anymore, this will continue working

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, I promised a test for this, let me add that

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added in 1538798

Copy link
Collaborator

@paula-stacho paula-stacho left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you! The title is actually a good idea, could we add it to the relationship items as well?

@gribnoysup gribnoysup merged commit da480de into main Aug 8, 2025
55 of 57 checks passed
@gribnoysup gribnoysup deleted the COMPASS-9653 branch August 8, 2025 13:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feat feature flagged PRs labeled with this label will not be included in the release notes of the next release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants